Skip to content

Ensure volume.read and volume.peek will always return the full amount of requested if available#33

Merged
Eeems merged 4 commits into
mainfrom
issue/18
May 15, 2026
Merged

Ensure volume.read and volume.peek will always return the full amount of requested if available#33
Eeems merged 4 commits into
mainfrom
issue/18

Conversation

@Eeems
Copy link
Copy Markdown
Owner

@Eeems Eeems commented May 14, 2026 •
edited by coderabbitai Bot
Loading

Summary by CodeRabbit

  • Bug Fixes

    • Improved read behavior to reliably accumulate requested data when underlying reads return partial data.
    • Fixed peek behavior to avoid altering the underlying stream position during peek operations.

  • Tests

    • Added tests simulating partial-read streams to validate resilient reading and peeking.

  • Chores

    • Bumped package version to 1.4.1.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026 •
edited
Loading

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7a88b806-42c9-4f33-90f6-c5e54b84064c

📥 Commits

Reviewing files that changed from the base of the PR and between 914bcfb and 6c4a810.

📒 Files selected for processing (1)
  • pyproject.toml
📝 Walkthrough

Walkthrough

Volume.read() and Volume.peek() now accumulate bytes in a loop to tolerate short reads from the underlying stream: read() advances the volume cursor by the actual bytes returned; peek() temporarily advances the underlying stream while accumulating and then restores the position so peek has no net side effects. Tests add a _ShortReadStream to exercise short-read behavior.

Changes

Volume Short-Read Resilience

Layer / File(s) Summary
Loop-based read with cursor advance
ext4/volume.py		 Volume.read(size) now repeatedly calls the underlying stream read until size bytes are accumulated or an empty read/EOF occurs; the result is truncated to size, the volume cursor is advanced by the number of bytes returned, and the bytes are returned.
Loop-based peek with position restore
ext4/volume.py		 Volume.peek(size) now repeatedly calls the underlying stream peek to accumulate up to size bytes. During accumulation it temporarily advances the underlying stream position for each chunk and, after truncating to size, seeks back to the original offset + cursor so the volume cursor and stream position are unchanged.
Short-read test fixture and test updates
test.py		 Adds _ShortReadStream wrapper (wraps PeekableStream) that intentionally returns fewer bytes for multi-byte reads to simulate short reads. Tests for test32.ext4/test64.ext4 construct ext4.Volume with this wrapper and adjust /test.txt assertions to check non-empty reads starting with b"hello world".
Project version bump
pyproject.toml		 Package version updated from 1.4 to 1.4.1.

Sequence Diagram

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

  • Eeems/python-ext4#33: Similar updates to Volume.read/Volume.peek for loop-based short-read handling and associated tests.

Poem

🐰 I nibble bytes that come in thin,
I loop again to gather them in.
Peek hops back where it began—
No stray seeks left by my plan.
Hooray for resilient reads within!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and specifically describes the main change: improving Volume.read and Volume.peek to return the full requested amount when available.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Eeems Eeems linked an issue May 14, 2026 that may be closed by this pull request
Volume does not expect stream.read() to return less data than requested if the data is available. #18
Closed
@Eeems Eeems marked this pull request as ready for review May 14, 2026 22:33
coderabbitai[bot]

This comment was marked as resolved.

Sign in to view

@Eeems

This comment was marked as resolved.

Sign in to view
@coderabbitai

This comment was marked as resolved.

Sign in to view
@Eeems Eeems changed the title Fix #18 Ensure volume.read and volume.peek will always return the full amount of requested if available May 15, 2026
@Eeems Eeems merged commit e259672 into main May 15, 2026
10 of 11 checks passed
@Eeems Eeems deleted the issue/18 branch May 15, 2026 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Volume does not expect stream.read() to return less data than requested if the data is available.

1 participant

@Eeems